workspace,ci: remove the os_input feature (make it unconditional) - #14902

Cursor Bugbot has reviewed your changes and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit d52c949. Configure here.}

Blobk

@itamar-starkware reviewed 65 files and all commit messages, and made 1 comment.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on itamar-starkware).

@yoavGrs made 1 comment.
Reviewable status: 65 of 66 files reviewed, 1 unresolved discussion (waiting on itamar-starkware).

a discussion (no related file):
Blocking

@yoavGrs reviewed 66 files and all commit messages, and made 7 comments.
Reviewable status: all files reviewed, 8 unresolved discussions (waiting on itamar-starkware).

crates/apollo_batcher/src/commitment_manager/commitment_manager_impl.rs line 576 at r4 (raw file):

                }
                CommitterRequestLabelValue::ReadPathsAndCommitBlock => {
                    // TODO(Ariel): Add dedicated metrics for the read-paths-and-commit endpoint.

Is something already missing for the TODO?

crates/apollo_committer/src/committer.rs line 493 at r4 (raw file):

}

impl<S, ForestDB> Committer<S, ForestDB>

Merge the two impl blocks.

crates/apollo_consensus_orchestrator/src/cende/central_objects_test.rs line 1172 at r4 (raw file):

    let rust_json = serde_json::to_value(rust_obj).unwrap();

    // `recent_state_commitment_infos` and `initial_reads` are os_input-only and absent from the

Update the comment

crates/apollo_consensus_orchestrator/src/cende/mod.rs line 107 at r4 (raw file):

    parent_proposal_commitment: Option<ProposalCommitment>,
    recent_block_hashes: Vec<BlockHashAndNumber>,
    // Defaulted on (testing-only) deserialization: blobs recorded before witness production omit

I didn't expect to see it in this PR.
Why is it needed?

crates/apollo_integration_tests/src/utils.rs line 617 at r4 (raw file):

            )
            .route(
                RECORDER_GET_COMMITMENT_INFOS_HEIGHT_OFFSET_PATH,

It would be better to enable the feature in the first phase and after that delete it, but NVM.
Please explain why it's acceptable for the recorder to always return null.

crates/blockifier/src/blockifier/transaction_executor.rs line 59 at r4 (raw file):

    Collect,
    /// Skips collection, leaving `BlockExecutionSummary::initial_reads` empty. Required when the
    /// state reader serves a read-set pre os_input feature.

Update the doc

echonet/echonet_types.py line 181 at r4 (raw file):

    """
    Configuration for running the Starknet OS over each received blob, via the
    block-hash CLI binary's `os run-os-stateless` subcommand.

Does it need the transaction_serde feature?

@itamar-starkware made 7 comments.
Reviewable status: all files reviewed, 8 unresolved discussions (waiting on yoavGrs).

crates/apollo_batcher/src/commitment_manager/commitment_manager_impl.rs line 576 at r4 (raw file):

I removed the TODO.
commirt block latencymetric is using both commit block variant (commit block and read paths and commit block). And this is good estimation of the latency of commiting block.

The TODO suggested separating the latency for each varient and it will create jumps in the latencies when syncing.

crates/apollo_committer/src/committer.rs line 493 at r4 (raw file):

Done.

crates/apollo_consensus_orchestrator/src/cende/central_objects_test.rs line 1172 at r4 (raw file):

Done.

crates/apollo_consensus_orchestrator/src/cende/mod.rs line 107 at r4 (raw file):

It's a hack that enable central_systest_blobs to store blobs in GCP without witnesses and initial reads.
This way we avoid merging 14910 PR (which add witnesses and initial read to the blobs)
This attributes will be deleted on the 14910 PR.

Another point on this one - since central_systest_blobs are sent without the witnesses and initial reads then we can bump their pin commit without code adjustments on this side.

It's ugly, but give us the ability to divide the real functionality and testing PRs.

crates/apollo_integration_tests/src/utils.rs line 617 at r4 (raw file):

It will always send the last 10 witnesses he has.
It will be fixed on integration_tests PRs.

crates/blockifier/src/blockifier/transaction_executor.rs line 59 at r4 (raw file):

Done.

echonet/echonet_types.py line 181 at r4 (raw file):

Yes it's needed, but not as a build flag.
the CLI unconditionally depends on starknet_os with deserialize features and then he pulls transaction_serde from blockifier.

So a plain cargo build -p starknet_committer_and_os_cli is working.

@yoavGrs reviewed 5 files and all commit messages, made 1 comment, and resolved 6 discussions.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on itamar-starkware).

crates/apollo_consensus_orchestrator/src/cende/mod.rs line 107 at r4 (raw file):

Okay, because it's very temporary.

@yoavGrs resolved 1 discussion.
Reviewable status: 65 of 66 files reviewed, 1 unresolved discussion (waiting on itamar-starkware).

@yoavGrs reviewed 1 file and all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on itamar-starkware).

@yoavGrs resolved 1 discussion.
Reviewable status: all files reviewed, 1 unresolved discussion.

@itamar-starkware made 1 comment and resolved 1 discussion.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on itamar-starkware).